Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the guidance on change history #1531

Open
wants to merge 31 commits into
base: 1.2-dev
Choose a base branch
from

Conversation

yolile
Copy link
Member

@yolile yolile commented Jun 24, 2022

Closes #1412

@yolile yolile requested a review from jpmckinney June 24, 2022 16:28
@yolile yolile requested a review from duncandewhurst July 4, 2022 17:57
docs/guidance/build/change_history_options.md Outdated Show resolved Hide resolved
docs/guidance/build/full_updates.md Outdated Show resolved Hide resolved
docs/guidance/build/incremental_updates.md Outdated Show resolved Hide resolved
docs/guidance/build/no_change_history.md Outdated Show resolved Hide resolved
docs/guidance/map/amendments.md Outdated Show resolved Hide resolved
docs/history/changelog.md Outdated Show resolved Hide resolved
docs/schema/merging.md Outdated Show resolved Hide resolved
docs/guidance/build/change_history_options.md Outdated Show resolved Hide resolved
docs/guidance/build/incremental_updates.md Outdated Show resolved Hide resolved
@yolile
Copy link
Member Author

yolile commented Aug 19, 2022

Thank you, @duncandewhurst, for the changes. I think this is ready for James to review again, right?
And should we add a link to the new page from the Determine data needs section? (particularly to the "Whether to publish a change history" item)

@jpmckinney
Copy link
Member

And should we add a link to the new page from the Determine data needs section? (particularly to the "Whether to publish a change history" item)

Yes!

Copy link
Member

@jpmckinney jpmckinney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rewrote change_history_options per the discussion here: #1531 (comment)

That page now uses "full updates" to mean "full updates in the context of a change history" – in contrast to incremental updates. It also uses "individual releases" to mean "individual releases, when you can't publish a change history" – in contrast to "compiled releases".

However, that usage is inconsistent with the other pages.

I suggest renaming the other pages:

  • Incremental updates -> Change history. As Duncan commented, the examples are a mix of full and incremental updates. In any case, we don't have a page for each of these (and I don't think we need one, as the distinction is fairly minor).
  • Full updates -> Individual releases with no change history
  • No change history -> Compiled releases with no change history

To be clear, if a publisher follows the 'easy releases' approach, then they aren't publishing a change history (at best, a third-party can reconstruct a change history).

There might need to be some corresponding changes to the introductory paragraphs on those other pages.

@duncandewhurst
Copy link
Contributor

@yolile, @jpmckinney are you happy for me to pick up and complete this PR?

@yolile
Copy link
Member Author

yolile commented Feb 28, 2024

@duncandewhurst Yes, I'm!

@duncandewhurst
Copy link
Contributor

duncandewhurst commented Mar 5, 2024

To-do:

  • Review built version of documentation
  • Check guidance on setting .date in guidance on hashing (no last modified date)
  • Cherry pick updates to amendments example and update JSON directives in example 3

@duncandewhurst
Copy link
Contributor

duncandewhurst commented Mar 8, 2024

@jpmckinney this is ready for your review. I've made the changes suggested in your last comment, simplified the examples in line with our discussions elsewhere, and made various copy edits for consistency and clarity.

Two things to flag:

  1. The old easy releases guidance describes how to use a hash function to update release id in the absence of a last modified date. However, it doesn't explain how to set date. In the new individual releases guidance, I've recommended setting date to the date the release was generated (i.e. the date the user downloaded the data). Happy to discuss if there might be a better approach.
  2. In the individual releases guidance, I've preserved the recommendation to prefix .id with the ocid so that users don't need to filter on ocid to find releases belonging to the same contracting process. However, I'm inclined to remove that recommendation so that the examples align with the rule that .id need only be unique within the scope of the contracting process. It seems like more work to filter on part of a field (.id) than on a separate field. What do you think?

Edit: I forgot to mention, this PR includes some commits from #1661 which is also awaiting review. Whichever is reviewed first, I'll make the necessary updates to the other one.

@jpmckinney
Copy link
Member

In the individual releases guidance, I've preserved the recommendation to prefix .id with the ocid so that users don't need to filter on ocid to find releases belonging to the same contracting process. However, I'm inclined to remove that recommendation so that the examples align with the rule that .id need only be unique within the scope of the contracting process. It seems like more work to filter on part of a field (.id) than on a separate field. What do you think?

Agree - I prefer your suggested simplification.

@duncandewhurst
Copy link
Contributor

Great. I've done that in c848382

@jpmckinney
Copy link
Member

jpmckinney commented Mar 27, 2024

Noting that amendments.md includes changes from #1661, but my suggested edits in that PR don't overlap, so there shouldn't be a merge conflict. amendments.md has an extra commits in this PR, which can be reviewed independently:

@duncandewhurst duncandewhurst self-assigned this Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Review in progress
Development

Successfully merging this pull request may close these issues.

Update the guidance on releases being optional on Record
3 participants